Skip to content

Add validation test for scripts that export main and fix changeset.js module pattern#7460

Merged
pelikhan merged 7 commits intomainfrom
copilot/fix-javascript-packaging
Dec 24, 2025
Merged

Add validation test for scripts that export main and fix changeset.js module pattern#7460
pelikhan merged 7 commits intomainfrom
copilot/fix-javascript-packaging

Conversation

Copy link
Contributor

Copilot AI commented Dec 24, 2025 •
edited
Loading

Added validation test to ensure scripts used with const { main } = require(...) pattern actually export a main function, preventing runtime errors. Fixed scripts/changeset.js to follow the correct module script pattern.

Changes:

  • Add validation test (main_export_validation_test.go) to ensure scripts export main when required with const { main } = require(...)
  • Fixed scripts/changeset.js: Updated to export main without calling it (module script pattern for bundling)
  • Updated scripts/changeset.test.js: Modified to invoke main via require() instead of direct script execution

Key Principle:
A script that exports main should never call main. The importer makes the call.

Pattern for module scripts (like scripts/changeset.js and those in pkg/workflow/js/):

async function main() {
  // ... async operations
}

module.exports = { main }; // Export only, no invocation
// The importer/bundler calls main when needed

How Bundling Works

When scripts are bundled for GitHub Actions inline execution:

  1. The bundler removes module.exports statements
  2. The bundler injects await main(); at the end
  3. The script executes in the GitHub Script context

Usage

For bundled/imported use (primary use case):

const { main } = require('./changeset.js');
await main();

For testing: The test file invokes main via require with proper process.argv setup.

Validation

Added main_export_validation_test.go that validates:

  • Scripts required with const { main } = require(...) actually export main
  • Scripts define the main function they export
  • Pattern compliance for scripts with main exports

This prevents runtime errors where code tries to destructure main from a module that doesn't export it.

All changeset tests pass with the updated invocation pattern.

Original prompt

Reference: https://github.com/githubnext/gh-aw/actions/runs/20476798244/job/58842775783#step:4:1

Fix JavaScript packaging and invocation

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@pelikhan
Copy link
Contributor

pelikhan commented Dec 24, 2025

@copilot check that required script export a main when trying to destructure main.

Copilot AI and others added 2 commits December 24, 2025 03:00
@pelikhan
Initial plan for fixing JavaScript packaging and invocation
b430af0 
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan
Fix changeset.js to export main and use await in inline execution
d439115 
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix JavaScript packaging and invocation Fix changeset.js to export main and use await in inline execution Dec 24, 2025
Copilot AI requested a review from pelikhan December 24, 2025 03:13
@pelikhan
Add validation test to ensure scripts export main when required
0c44fba 
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 24, 2025

@copilot check that required script export a main when trying to destructure main.

Added validation test in commit 0c44fba that checks all scripts used with const { main } = require(...) actually export main. The test validates:

  1. Scripts required with main destructuring export a main function
  2. Scripts define the main function they export
  3. Pattern compliance for scripts with main exports

All currently required scripts pass validation.

@pelikhan
Copy link
Contributor

pelikhan commented Dec 24, 2025

Remove the direct invocations like

if (require.main === module) {
(async () => {
await main();
})();
}

@pelikhan
Copy link
Contributor

pelikhan commented Dec 24, 2025

@copilot comment above

@pelikhan
Remove require.main check and use unconditional async IIFE execution
f2601f3 
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 24, 2025 •
edited
Loading

@copilot comment above

Done in commit f2601f3.

Remove the direct invocations like

if (require.main === module) {...

Removed the if (require.main === module) check in commit f2601f3. The script now exports main and executes it unconditionally with an async IIFE:

module.exports = { main };

(async () => {
  await main();
})();

All tests pass with this pattern.

@pelikhan
Copy link
Contributor

pelikhan commented Dec 24, 2025

@copilot a script that exports main should never call main. The importer makes the call.

@pelikhan
Revert to original pattern: remove export and call main directly
ebfd7c4 
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 24, 2025

@copilot a script that exports main should never call main. The importer makes the call.

Understood. Reverted in commit ebfd7c4. The standalone CLI script scripts/changeset.js now:

  • Does NOT export main
  • Calls main() directly
  • Follows the principle: scripts that export main should never call main

Module scripts in pkg/workflow/js/ export main without calling it, letting the importer make the call.

Copilot AI changed the title Fix changeset.js to export main and use await in inline execution Add validation test for scripts that export main Dec 24, 2025
@pelikhan
Copy link
Contributor

pelikhan commented Dec 24, 2025

@copilot changeset is a module script. It gets imported in actions/github-script and main is run in the inlined JavaScript.

Changeset must use export main and not call it.

@pelikhan
Export main without calling it, update test to invoke via require
579ef57 
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 24, 2025

@copilot changeset is a module script. It gets imported in actions/github-script and main is run in the inlined JavaScript.

Changeset must use export main and not call it.

Fixed in commit 579ef57. scripts/changeset.js now:

  • Exports main via module.exports = { main }
  • Does NOT call main
  • Test updated to invoke via require() and call main

The bundler will remove exports and inject await main(); for inline GitHub Script execution.

Copilot AI changed the title Add validation test for scripts that export main Add validation test for scripts that export main and fix changeset.js module pattern Dec 24, 2025
@pelikhan pelikhan marked this pull request as ready for review December 24, 2025 04:32
@pelikhan pelikhan merged commit b9cce4d into main Dec 24, 2025
4 checks passed
@pelikhan pelikhan deleted the copilot/fix-javascript-packaging branch December 24, 2025 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@pelikhan pelikhan pelikhan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@pelikhan pelikhan

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pelikhan